This would destroy all data in BigBuf[] when reconnecting. I think the major purpose of the possibility to reconnect is to support offline functionality like sniffing some data?

As it is now, unplugging causes the CPU to shoot to 100% used and the only way to continue what you're doing is to exit and restart the client. When I'm on my laptop I sometimes need to move USB ports or occasionally it accidentally comes unplugged, and needing to restart is annoying.

At any rate I've seen some odd segfaults in readline with this, so I was thinking about ripping out the "re-get version info" stuff anyway.

Also, as far as I can tell clearing the command buf doesn't touch BigBuf[]. Not sure if the act of disconnecting/reconnecting the USB cable will do that, though without a battery whatever's in the hardware will be gone.

Indeed, clearCommandBuffer() doesn't touch BigBuf[] but querying for the version info does. This was the main reason to cache the version info on client side. I see two possible solutions:

• cache the version info on device side instead of client side
• don't query the version info on reconnect

The latter is obviously easier to implement.

Newest version still has clearCommandBuffer() but instead of querying the version on re-connect it simply clears CmdVersion()'s cache.

Hey, can you please hold up on this until #463 is in? Aside from not wanting to have to go through a merge conflict process again, I address some of the reconnection issues already in that for flasher.

This will break on Android. Is there a better way to pass this from within the main() function instead?

See https://github.com/AndProx/AndProx/blob/b662c653d7ba33b570d71e34c949e3a8e7f17d8e/natives/src/main/cpp/natives.c#L82

Currently, this assumes that the "serial port identifier will always be a string". This isn't true if uart_open is replaced.

• For each of uart_win32.c and uart_posix.c, add a char* referring to the serial port path in their platform-specific serial_port structs.

• In uart.h, add a method bool uart_reconnect(serial_port* sp).

• uart_reconnect can then be implemented in platform-specific ways, that takes in the serial port identifier stashed away earlier, and will setup the existing serial_port struct to work again. This returns true on success, or false on error.

• Whenever the application determines the need to reconnect, it calls uart_reconnect, which can operate in-place. uart_receiver can have some retry logic.

I have no idea what part of the above code will fail on Android, that link doesn't explain it. The original PM3 code takes main()'s argv[1] and feeds it to uart_open(). sp_name is just a copy of argv[1]. In what cases will argv[1] not be a char* ? If main() is changed to not feed argv[1] into uart_open then obviously the other instances of uart_open() will need to be changed too. Surely replacing "char*" with whatever becomes appropriate at that time isn't that difficult...

The entire point of this patch is to simply detect a disconnection and attempt to reopen the same port. It is not to rewrite the entire uart system.

All of it will fail on Android, because the "serial port" is a reference to a bunch of JNI object IDs, which allow it to pass uart_send and uart_receive to Java method calls.

This is implemented in these files:

• https://github.com/AndProx/AndProx/blob/ae58e617cfd033f2d3ac94046a89291c790002a2/natives/src/main/cpp/natives.c#L122-L214
• https://github.com/AndProx/AndProx/blob/master/natives/src/main/cpp/uart_android.c
• https://github.com/AndProx/AndProx/blob/master/natives/src/main/java/au/id/micolous/andprox/natives/NativeSerialWrapper.java

PM3 doesn't get called by main either, so there is no argv. The link I sent is one of the bootstrap functions which sets up PM3 similar to how main would.

This is done because there is no /dev/ttyACM0 except on rooted devices where one has installed the cdc_acm kernel module. Everything is passed around the Android USB Host API. Requiring root is a major blocker to use on the platform.

Having an API call for "reconnect" means that it doesn't matter how uart is implemented, whether it's a string pointing to a device name, it's wrapped up in some other platform specific code, or it's part of some testing harness.

At least on Android I'd bring "reconnect" up into Java space, hit the USB stack on the head a few times, and then pass it back.

As for precedent: @iceman1001 objected when I proposed removing uart_{get,set}_speed even though that's not actually used in mainline PM3, but was in his branch.

If that went in as-is, even when it was "internal", I would no longer be able to track upstream PM3 for Android until it was replaced with the API extensions I proposed.

Why does this path also return true? Don't you want to know when there is an error?

Timing out without receiving any data may not be an error if the hardware just doesn't have any data ready for us at the moment.

uart_posix.c does some additional and unnecessary re-packaging which is already handled in proxmark3.c uart_receiver():

			if (prx-rx < sizeof(UsbCommand)) {
				continue;
			}

uart_win32.c doesn't do that. Why not aligning/simplyfying the Unix version and make it behave like the Windows version?

Ah true. I also double checked ReadFile (the win32api function that this is equivalent to), that doesn't return false on timeout.

So that entire if statement can go.

The Win and *nix APIs are totally different. It's been a while since I looked at uart_posix.c and I really didn't spend a huge amount of time in there, but from what I recall the various return's are to break out of a loop or to handle the various conditions the select() can result in (read error vs "no data available" which isn't an error). If it's rewritten to be like uart_win32.c then it will block forever (or at least until the PM3 is unplugged) and not allow the comms thread to exit on client shutdown due to the "do { ... } while(byteCount);" . The only clean-up I see is the "if(..) return true; else return true;" As per my first comment I have absolutely no clue if the WIN32 routine returns an error on no data or not, or an error on a read error or not. No windoze boxen in this building...

FYI, if you want to test stuff on Windows, http://modern.ie/ has time-limited trial Windows virtual machines. They're ostensibly for testing websites in Internet Explorer and Edge, but you can install any software you like on them.

That should be sufficient for getting ProxSpace (the Windows build environment) going.